You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Support relative and absolute path keys in fileWindows lookup
Summary
• In sdk/src/tools/read-files.ts, resolve fileWindows using filePath, relativePath, or fullPath.
• Previously, windows was looked up strictly via fileWindows?.[filePath]. If filePaths contained an alias (e.g. ./src/file.ts or an absolute path /cwd/src/file.ts) while fileWindows was keyed by the canonical relativePath (src/file.ts), or vice versa, the lookup evaluated to undefined.
• When windows is undefined while fileWindows is enabled, the code defaulted to [{}], dumping the whole file from line 1 and discarding the model's requested window offset/limit.
• Adding fallback lookups for fileWindows?.[relativePath] and fileWindows?.[fullPath] ensures requested windows are accurately applied regardless of path format.
• Adds regression tests in sdk/src/__tests__/read-files.test.ts verifying window resolution across relative, dot-slash, and absolute path variations.
Test plan
[✓] bun test src/__tests__/read-files.test.ts — 40 pass, 0 fail
[✓] bun run --cwd sdk typecheck — 0 errors
[✓] PR hygiene check passed
Good catch and a clean, minimal fix. The core issue is real: fileWindows is keyed inconsistently relative to how filePaths may be passed in (dot-relative, bare-relative, or absolute), and a strict [filePath] lookup silently falls through to [{}], which discards the caller's requested offset/limit and dumps the whole file. That's a legitimate bug with observable impact.
The fix in sdk/src/tools/read-files.ts (lines ~116-122) adds fallback lookups against relativePath and fullPath, which covers the three cases demonstrated in the new tests. The three added tests in read-files.test.ts are well-targeted and each isolate one path-format mismatch, which is what I'd want to see for a fix like this.
One thing worth flagging for review: the precedence order is filePath -> relativePath -> fullPath. If two different keys in fileWindows happen to both match (e.g. via aliasing), the first non-nullish one wins silently; that's probably fine in practice since normal callers wouldn't set conflicting entries, but it's worth a maintainer's eye. Also worth checking whether relativePath/fullPath are always defined at this point in the function (the fullPath ? guard suggests not always), so it'd be good to confirm no other callers rely on the old strict-only behavior.
Overall this is a small, correct, well-tested change to a shared SDK path-resolution helper — worth porting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
codebuff-team
added
bot:triaged
Support relative and absolute path keys in fileWindows lookup
Summary
• In
sdk/src/tools/read-files.ts, resolvefileWindowsusingfilePath,relativePath, orfullPath.• Previously,
windowswas looked up strictly viafileWindows?.[filePath]. IffilePathscontained an alias (e.g../src/file.tsor an absolute path/cwd/src/file.ts) whilefileWindowswas keyed by the canonicalrelativePath(src/file.ts), or vice versa, the lookup evaluated toundefined.• When
windowsisundefinedwhilefileWindowsis enabled, the code defaulted to[{}], dumping the whole file from line 1 and discarding the model's requested window offset/limit.• Adding fallback lookups for
fileWindows?.[relativePath]andfileWindows?.[fullPath]ensures requested windows are accurately applied regardless of path format.• Adds regression tests in
sdk/src/__tests__/read-files.test.tsverifying window resolution across relative, dot-slash, and absolute path variations.Test plan
[✓]
bun test src/__tests__/read-files.test.ts— 40 pass, 0 fail[✓]
bun run --cwd sdk typecheck— 0 errors[✓] PR hygiene check passed